Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WMBUS extension to dispatch messages - old discussion #1248

Closed
wants to merge 42 commits into from

Conversation

HomeAutoUser
Copy link
Contributor

@HomeAutoUser HomeAutoUser commented Apr 3, 2024

  • Please check if the PR fulfills these requirements
  • Tests for the changes have been added / modified (needed for for bug fixes / features)
  • commandref has been added / updated (needed for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  • What is the current behavior?
    (You can also link to an open issue here, if this describes the current behavior)
    -- no dispatch

  • What is the new behavior (if this is a feature change)?
    -- you can dispatch a WMBUS message to clientmodule

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • Other information:
    -- in order to receive messages yourself, the firmware must be adapted (later step)



my $mark;
if (substr($rmsg,5,1) eq 'Y') { # WMBus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich verstehe grob, warum das Y hier entfernt wird.
Was ich aber nicht verstanden habe ist, wieso die Daten mit Y und nicht MN im Modul ankommen.
Gerade auch, weil die FW ja noch angepasst werden muss.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MIST falsche Nachricht gelöscht :-(

Copy link
Contributor Author

@HomeAutoUser HomeAutoUser Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was ich aber nicht verstanden habe ist, wieso die Daten mit Y und nicht MN im Modul ankommen.
Gerade auch, weil die FW ja noch angepasst werden muss.

Ich schreibe nochmal "nur" kurz die Antwort.
Das mit dem Y wird aus der Historie und der Vielfahlt des WMBus Protokolles entstanden sein.
Wegen der Kompatiblität müsste es so beibehalten bleiben weil es so der CUL empfängt.

CUL_RAWMSG | bY25442D2C628346771B168D20BE124C0E20D48C96EA64C8FCCC14A07A3A8840A30DE4F084AB8D80F7
or
CUL_868Mhz_RAWMSG | b3E44F536892656000108F5D97AAC0020256B755DD9228B433D8D5C82F0618CAD0643C2E540A722C2DC9B8A0138CE0041A2DCD11DE10F800C0001090086B41E0098DB14011E070416C580
or
signalGateway_DMSG | b3E44F536892656000108F5D97AA9002025FB7AD4DF8710D69A3867E687B0633B1515F7BA8A6BE0C93649EC5593F476E7DD3E82F0470F800C0001090086B41E0098CE14011E070416C50018
signalGateway_Protocol_ID | 134
signalGateway_RAWMSG | MN;D=3E44F536892656000108F5D97AA9002025FB7AD4DF8710D69A3867E687B0633B1515F7BA8A6BE0C93649EC5593F476E7DD3E82F0470F800C0001090086B41E0098CE14011E070416C50018;R=24;A=0;
or
signalGateway_DMSG | bY304497264202231800087A5E0020A5D9CF9E719E36DB255C06F2AEC0722FC3853A3031BD85EF085BDAD29194136A02DD7E00FD
signalGateway_Protocol_ID | 134
signalGateway_RAWMSG | MN;D=304497264202231800087A5E0020A5D9CF9E719E36DB255C06F2AEC0722FC3853A3031BD85EF085BDAD29194136A02DD7E00FD;R=253;A=0;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, das leuchtet mir noch nicht ein.
Das logische Modul braucht die Daten wie sie auch das CUL Modul übergibt.

Aber das SIGNALduino Modul braucht sie doch nur so, wie es der SIGNALduino übergibt.

Funktioniert beim IT Modul doch auch genau so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Habe die RAW MSGs erst jetzt gesehen, aber wenn ich es richtig verstehe würde der SIGNALduino wie sonst auch

MN;D=304497264202231800087A5E0020A5D9CF9E719E36DB255C06F2AEC0722FC3853A3031BD85EF085BDAD29194136A02DD7E00FD;R=253;A=0;
übergeben.
Das bedeutet aber auch, dass hier nicht Y vom uC an das Modul übergeben wird, sondern der Y vom Modul an das logische Modul.. Richtig? In "SIGNALduino_Parse_MN" würde somit nie das gesuchte Y in der Variable rmsg ankommen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naja, wenn das Y vom Uc kommt, dann hätte ich vorgeschlagen die Regex anzupassen.
Den Teil der der an _limit_to_hex hätte ich mit einem substr von dem ersten Zeichen (das Y) entfernt.

Dann bleibt das Y in der rmsg enthalten.

Wäre aber schön, wenn wir einen Testdatensatz (rmsg) mit dem Y aufnehmen.
Die fehlt da und machte mich halt unsicher ob das vom uC so kommt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich habe mal Euch etwas zusammengestellt @sidey79 & @elektron-bbs

-MSG1-
uC_RAWMSG
            MN;D=Y304497264202231800087AE30020A5C247707714E518602D984FE2BF82C023288C39460740DC47132AAE42791B8F48F2EB00FD;R=253;A=0;
Internal_DMSG
                bY304497264202231800087AE30020A5C247707714E518602D984FE2BF82C023288C39460740DC47132AAE42791B8F48F2EB00FD
Internal_RAMSG
             MN;D=304497264202231800087AE30020A5C247707714E518602D984FE2BF82C023288C39460740DC47132AAE42791B8F48F2EB00FD;R=253;A=0

-MSG2-
uC_RAWMSG
            MN;D=Y304497264202231800087AE40020A5D75DA00DA8A5A3E0E7A39CEFE3FA26FDC78E6C874675EFA951F6031F8E211C9E0BA500F7;R=247;A=0;
HTML_RAWMSG (uninteressant für das SIGNALduino Projekt)
                 Y304497264202231800087AE40020A5D75DA00DA8A5A3E0E7A39CEFE3FA26FDC78E6C874675EFA951F6031F8E211C9E0BA5
Internal_DMSG
                bY304497264202231800087AE40020A5D75DA00DA8A5A3E0E7A39CEFE3FA26FDC78E6C874675EFA951F6031F8E211C9E0BA500F7
Internal_RAMSG
             MN;D=304497264202231800087AE40020A5D75DA00DA8A5A3E0E7A39CEFE3FA26FDC78E6C874675EFA951F6031F8E211C9E0BA500F7;R=247;A=0;

Wie verfahren wir weiter oder wie gedenkst du @sidey79 solche Nachrichten an das entsprechende Modul weiterzugeben?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was stört dich jetzt eigentlich an den Nachrichten?
Das einzige, was mir auffällt, ist das "Internal_RAMSG", das nicht der Nachricht vom µC entspricht.
Oder werden die Nachrichten etwa gar nicht an das WMBus-Modul weitergegeben?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich bin immer noch dafür, dass eine rmsg Einzug in die Testdaten erhält.
Dann können wir die Implementierung so anpassen, dass die Tests erfolgreich sind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sidey79

Ich bin immer noch dafür, dass eine rmsg Einzug in die Testdaten erhält.
Dann können wir die Implementierung so anpassen, dass die Tests erfolgreich sind.

https://github.com/RFD-FHEM/RFFHEM/pull/1248/files#diff-092416c5806fd44b768736cdcc4ed6b8b54b0c3f6123d65e96e3cc6f41b38c86R143-R174

@elektron-bbs

Oder werden die Nachrichten etwa gar nicht an das WMBus-Modul weitergegeben?
Doch diese werden an das Modul weitergegeben aber bei jeder _RMSG ist das Phänomen so.

HomeAutoUser and others added 3 commits April 4, 2024 17:54
Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.92%. Comparing base (8bf9724) to head (52d0be7).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1248      +/-   ##
==========================================
- Coverage   89.13%   88.92%   -0.21%     
==========================================
  Files          43       43              
  Lines        2466     2466              
  Branches      170      170              
==========================================
- Hits         2198     2193       -5     
- Misses        114      119       +5     
  Partials      154      154              
Flag Coverage Δ
fhem 88.92% <ø> (-0.21%) ⬇️
modules 88.92% <ø> (-0.21%) ⬇️
perl 88.92% <ø> (-0.21%) ⬇️
unittests 88.92% <ø> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* revised datarate
elektron-bbs
elektron-bbs previously approved these changes Apr 7, 2024
deviceName => q[dummyDuino],
plan => 2,
testname => q[Good MN data, with RSSI, with set attribute rfmode=WMBus_T, DMSG started with bY],
input => q[MN;D=304497264202231800087A5E0020A5D9CF9E719E36DB255C06F2AEC0722FC3853A3031BD85EF085BDAD29194136A02DD7E00FD;R=253;A=0;],
Copy link
Contributor Author

@HomeAutoUser HomeAutoUser Apr 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sidey79 , diese MN Message ergibt eine DMSG mit bY. (Glaube ich, da ich den Kommentar so definierte)
PS: Genaue Daten habe ich erst morgen wieder am PC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Es muss nach den Erklärungen doch auch eine RMSG mit MN;D=Y geben.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Es wird nun noch von Vorteil sein, bei diesen Warnings

2024-04-09T21:36:45.3459830Z 2024.04.09 21:36:39 4: dummyDuino: Parse_MN, Found 2-FSK Protocol id 108 -> Bresser 5in1
2024-04-09T21:36:45.3461571Z 2024.04.09 21:36:39 1: PERL WARNING: Illegal hexadecimal digit 'Y' ignored at FHEM/lib/SD_Protocols.pm line 1952.
2024-04-09T21:36:45.3483159Z 2024.04.09 21:36:39 4: dummyDuino: Parse_MN, Found 2-FSK Protocol id 117 -> Bresser 7in1
2024-04-09T21:36:45.3484865Z 2024.04.09 21:36:39 1: PERL WARNING: Illegal hexadecimal digit 'Y' ignored at FHEM/lib/SD_Protocols.pm line 2029.
2024-04-09T21:36:45.3505768Z 2024.04.09 21:36:39 4: dummyDuino: Parse_MN, Found 2-FSK Protocol id 131 -> Bresser lightning
2024-04-09T21:36:45.3508364Z 2024.04.09 21:36:39 1: PERL WARNING: Illegal hexadecimal digit 'Y' ignored at FHEM/lib/SD_Protocols.pm line 2052.

in der Datei SD_ProtocolData.pm jeweils bei der ID 108,117,131 den regexMatch => qr/^[a-fA-F0-9]+/, zu ergänzen. So würden doch die DMSG Nachrichten mit Y keinen Fehler produzieren. Oder sehe ich das verkehrt?

Comment on lines 2912 to 2916
if (substr($rmsg,5,1) eq 'Y') { # WMBus frame type B
$rmsg1 = $rmsg;
$rmsg =~ s/^MN;D=Y/MN;D=/;
$mark = 'Y';
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sidey79 wir haben hier mal das RegEx eindeutig gestaltet um das Y zu entfernen. Die Zeichenkette MN;D=Y kommt nur einmal hervor in der RAWMSG und so kann es zu keiner Verwechslung kommen.

Zusätzlich haben wir mal intensiver nachgesehen weil dir das entfernen nicht so richtig zusagt.
Wir sind der Meinung, das wäre

    1. aufwending
    1. wiedersprichst du den Funktionsnamen (LimitHex)
    1. erhöhst du ggf auch die Chance fehlerhafter Nachrichten verarbeitet zu werden.

Im Klartext:
Sobald du das Y in die Prüfungen einbauen möchtest, dann muss es an 3 Stellen (Routinen eingearbeitet werden was sehr arbeitsreich ist).

  • SIGNALduino_Parse_MN -> rmsg Prüfung
  • SIGNALduino_Split_Message Funktion (wird in SIGNALduino_Parse_MN aufgerufen)
  • _limit_to_hex Funktion (wird in SIGNALduino_Parse_MN aufgerufen)
  • _limit_to_hex Funktion (hier ein Y zuzulassen wäre gegen den Name HEX-Prüfung)
  • wir erhöhen somit auch die Chance, das "falsche MSGs" woanders verarbeitet werden könnten --> ggf. andere Fehler oder Warnings

Können wir dies so beibehalten wie von uns vorgeschlagen?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich denke da liegt ein Missverständnis vor.

Ich habe mir das ganz einfach vorgestellt.

Wie könnte ich das einfach bereitstellen.. hmm auf diesen PR kann ich ja keinen eigenen gescheit machen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wie könnte ich das einfach bereitstellen.. hmm auf diesen PR kann ich ja keinen eigenen gescheit machen.

Meinst du damit, keine Kopie des Branches um darin zu arbeiten oder etwas anderes?
Das könnte ich in das RFHEM Projekt duplizieren in einen separaten Branch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Können wir so machen, aber heute schaffe ich es nicht mehr.
Bin total fertig....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sidey79 , du hast nun hier https://github.com/RFD-FHEM/RFFHEM/tree/master_WMBUS freie Hand ;-) um ggf deine Vorstellungen umzusetzen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich habe es gepusht

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vielen Dank @sidey79.
Ich hoffe das Ganze richtig vereint zu haben hier, oder soll 7 wollen wir einen neuen PR von dem Branch, wo du geändert hast erstellen?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wäre vermutlich die bessere Alternative, da dann auch der Codecover Report geht.

@HomeAutoUser HomeAutoUser changed the title WMBUS extension to dispatch messages WMBUS extension to dispatch messages - old discussion Apr 22, 2024
@HomeAutoUser HomeAutoUser deleted the master_WMBUS branch April 25, 2024 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants